Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mutex #499

Merged
merged 12 commits into from
Oct 28, 2024
Merged

Mutex #499

merged 12 commits into from
Oct 28, 2024

Conversation

roxblnfk
Copy link
Collaborator

@roxblnfk roxblnfk commented Sep 9, 2024

What was changed

  • Added a new class Mutex.
    The Mutex class has a lock() method that returns a Promise. This means yield $mutex->lock() will wait for the lock is acquired.
    The Mutex class also has tryLock(), unlock(), and isLocked() methods, which do not imply Await behavior.

  • Added Workflow::runLocked(Mutex $mutex, callable $callable): PromiseInterface:
    Executes a function if the given Mutex is not locked and locks the Mutex for the duration of the function's execution.

Checklist

An explanation

use Temporal\Workflow;

#[Workflow\WorkflowInterface]
class TestWorkflow
{
    #[Workflow\WorkflowMethod]
    public function handle(): \Generator
    {
        $mutex = new Workflow\Mutex();

        // Wait for the Mutex to be unlocked
        yield $mutex;
        assert(false === $mutex->isLocked(), 'The Mutex is unlocked here');

        // Try to lock the Mutex, returns true if the Mutex was successfully locked
        assert(true === $mutex->tryLock());

        // Try to lock the Mutex again, returns false because the Mutex is already locked
        assert(false === $mutex->tryLock());

        // We may use it in Workflow::await() and Workflow::awaitWithTimeout() like a condition.
        // It means wait until the Mutex is unlocked or timeout.
        yield Workflow::awaitWithTimeout('5 seconds', $mutex);
        assert(true === $mutex->isLocked(), 'The Mutex is still locked here because of the timeout');
        $mutex->unlock(); // Unlock for the next test

        // Get the mutex locked state and unlock it after the function is finished.
        // The function will be executed asynchronously, so we can use `yield` inside to wait for the result.
        // We don't need to control the Mutex manually, it will be locked and unlocked automatically.
        yield Workflow::runLocked($mutex, static function () {
            // Mutex is locked here
            yield Workflow::timer('1 second');
            // Mutex is still locked here
        });
        assert(false === $mutex->isLocked(), 'Mutex is unlocked here');

        // In this example, we run 5 functions in parallel, but only one function can be executed at a time.
        // When the first function is finished, the next function will be executed.
        // All functions will be executed in the order they were added.
        yield \Temporal\Promise::all([
            Workflow::runLocked($mutex, $this->doSomething()),
            Workflow::runLocked($mutex, $this->doSomething()),
            Workflow::runLocked($mutex, $this->doSomething()),
            Workflow::runLocked($mutex, $this->doSomething()),
            Workflow::runLocked($mutex, $this->doSomething()),
        ]);

        // Additionally, you can cancel the Promise returned by `runLocked` to interrupt or discard the function.
        $promise = Workflow::runLocked($mutex, $this->doSomethingSomeTime());
        $promise->cancel();
    }
}

Copy link

vercel bot commented Sep 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
php ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 28, 2024 2:40pm

@roxblnfk roxblnfk linked an issue Sep 9, 2024 that may be closed by this pull request
@roxblnfk roxblnfk added the Feature New feature or request label Sep 9, 2024
@wolfy-j
Copy link
Collaborator

wolfy-j commented Sep 9, 2024

Maybe we can make name optional?

@wolfy-j
Copy link
Collaborator

wolfy-j commented Sep 9, 2024

I would avoid having conditional mutexes, we can always achieve that by using await + mutex

@Sushisource
Copy link
Member

Sushisource commented Sep 9, 2024

Maybe we can make name optional?

I would avoid having conditional mutexes, we can always achieve that by using await + mutex

+1 to both of these. I was going to say don't do name at all, but then I saw PHP's built-in mutex takes a name which I find really odd, but, I agree it makes sense to match that API.

As for the conditional, yeah, user can always use await & we haven't provided that in other langs either so we can stick with the basic APIs.

The only other thing I would add is a tryLock method which the other SDKs support.

@roxblnfk
Copy link
Collaborator Author

I was going to say don't do name at all, but then I saw PHP's built-in mutex takes a name which I find really odd

We can definitely get rid of the name. SyncMutex is not a PHP standard; it's just a third-party extension, and we don't have to consider it.

@roxblnfk
Copy link
Collaborator Author

roxblnfk commented Sep 10, 2024

PR updated according to the review


If we don't use the mutex name, we can simplify everything:

  • Remove the MutexInterface
  • Remove the Workflow::mutex() method, and create Mutex using the constructor new Mutex()

@Sushisource
Copy link
Member

This approach is looking good to me 👍

# Conflicts:
#	src/Workflow/WorkflowContextInterface.php
@roxblnfk roxblnfk marked this pull request as ready for review October 8, 2024 07:24
@roxblnfk roxblnfk marked this pull request as draft October 8, 2024 11:57
@roxblnfk roxblnfk marked this pull request as ready for review October 9, 2024 18:47
@roxblnfk roxblnfk changed the title The concept of mutexes and locks Mutex Oct 11, 2024
@roxblnfk roxblnfk added this to the 2.12.0 milestone Oct 28, 2024
@roxblnfk roxblnfk merged commit a1a6c59 into master Oct 28, 2024
93 checks passed
@roxblnfk roxblnfk deleted the mutex branch October 28, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow-friendly concurrency control
3 participants